[SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS SELECT by clockfly · Pull Request #13451 · apache/spark

clockfly · 2016-06-01T20:32:54Z

What changes were proposed in this pull request?

This PR bans syntax like CREATE TEMPORARY TABLE USING AS SELECT

CREATE TEMPORARY TABLE ... USING ... AS ... is not properly implemented, the temporary data is not cleaned up when the session exits. Before a full fix, we probably should ban this syntax.

This PR only impact syntax like CREATE TEMPORARY TABLE ... USING ... AS ....
Other syntax like CREATE TEMPORARY TABLE .. USING ... and CREATE TABLE ... USING ... are not impacted.

How was this patch tested?

Unit test.

andrewor14 · 2016-06-01T20:46:38Z

Looks good. Can you add a test for the new exception that you throw?

SparkQA · 2016-06-01T22:15:55Z

Test build #59770 has finished for PR 13451 at commit fa1db6a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-01T23:31:18Z

Test build #59776 has finished for PR 13451 at commit 9bedd70.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2016-06-02T01:13:27Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala

Can you move this test case to HiveDDLCommandSuite, which contains the Hive-specific parser test cases? You also can see the other CTAS test cases there. Thanks!

@gatorsmile Thanks. The test should be included in CreateTableAsSelectSuite

HiveDDLCommandSuite contains "create table as select", CreateTableAsSelectSuite contains "create table ... using ... as select ..."

Normally, parser-specific test cases are put in the same suite. I am fine if you put it to the CTAS-specific test suite, especially when we will support it in the near future.

clockfly · 2016-06-02T06:22:31Z

cc @cloud-fan @liancheng @andrewor14

gatorsmile · 2016-06-02T06:25:00Z

sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala

+
+      checkAnswer(
+        sql("SELECT a, b FROM jsonTable"),
+        sql("SELECT a, b FROM jt").collect())


Nit: you do not need to call collect

cloud-fan · 2016-06-02T06:37:43Z

LGTM except some minor issues which are already pointed out by others.

clockfly · 2016-06-02T06:39:47Z

Thanks @gatorsmile

gatorsmile · 2016-06-02T06:46:43Z

LGTM. Thanks!

SparkQA · 2016-06-02T06:54:14Z

Test build #59818 has finished for PR 13451 at commit d4f5ab6.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2016-06-02T07:12:38Z

Test build #59822 has finished for PR 13451 at commit b7bb43d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-02T07:53:21Z

Test build #59828 has finished for PR 13451 at commit 90fb80a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-02T08:08:00Z

Test build #59830 has finished for PR 13451 at commit 2f0355f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-02T08:31:44Z

Test build #59834 has finished for PR 13451 at commit aaca7f0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

tejasapatil · 2016-06-02T14:44:05Z

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala

      // Get the backing query.
      val query = plan(ctx.query)

+      if (temp) {


nit: can you please rename this to temporary

Probably not in the scope of this PR, temp is a existing variable defined at https://github.com/clockfly/spark/blob/aaca7f01882de19c9dda47500f7d4f03712d3383/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala#L304.

liancheng · 2016-06-02T20:38:42Z

@andrewor14 Could you please help sign off this one? Thanks!

andrewor14 · 2016-06-02T21:10:02Z

LGTM!

andrewor14 · 2016-06-02T21:10:29Z

Merging into amster 2.0

## What changes were proposed in this pull request? This PR bans syntax like `CREATE TEMPORARY TABLE USING AS SELECT` `CREATE TEMPORARY TABLE ... USING ... AS ...` is not properly implemented, the temporary data is not cleaned up when the session exits. Before a full fix, we probably should ban this syntax. This PR only impact syntax like `CREATE TEMPORARY TABLE ... USING ... AS ...`. Other syntax like `CREATE TEMPORARY TABLE .. USING ...` and `CREATE TABLE ... USING ...` are not impacted. ## How was this patch tested? Unit test. Author: Sean Zhong <seanzhong@databricks.com> Closes #13451 from clockfly/ban_create_temp_table_using_as. (cherry picked from commit d109a1b) Signed-off-by: Andrew Or <andrew@databricks.com>

clockfly force-pushed the ban_create_temp_table_using_as branch from e48caf0 to fa1db6a Compare June 1, 2016 20:35

gatorsmile reviewed Jun 2, 2016
View reviewed changes

clockfly force-pushed the ban_create_temp_table_using_as branch 5 times, most recently from b7bb43d to 90fb80a Compare June 2, 2016 05:54

clockfly changed the title ~~[SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING AS SELECT~~ [SPARK-15711][SQL]Ban CREATE TEMPORARY TABLE USING AS SELECT Jun 2, 2016

clockfly changed the title ~~[SPARK-15711][SQL]Ban CREATE TEMPORARY TABLE USING AS SELECT~~ [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS SELECT Jun 2, 2016

Ban CREATE TEMP TABLE USING AS SELECT for now

2f0355f

clockfly force-pushed the ban_create_temp_table_using_as branch from 90fb80a to 2f0355f Compare June 2, 2016 06:07

gatorsmile reviewed Jun 2, 2016
View reviewed changes

fix the exception type.

a94dd89

fix style.

aaca7f0

tejasapatil reviewed Jun 2, 2016
View reviewed changes

asfgit closed this in d109a1b Jun 2, 2016

Conversation

clockfly commented Jun 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

andrewor14 commented Jun 1, 2016

Uh oh!

SparkQA commented Jun 1, 2016

Uh oh!

SparkQA commented Jun 1, 2016

Uh oh!

gatorsmile Jun 2, 2016

Choose a reason for hiding this comment

Uh oh!

clockfly Jun 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile Jun 2, 2016

Choose a reason for hiding this comment

Uh oh!

clockfly commented Jun 2, 2016

Uh oh!

gatorsmile Jun 2, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jun 2, 2016

Uh oh!

clockfly commented Jun 2, 2016

Uh oh!

gatorsmile commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

tejasapatil Jun 2, 2016

Choose a reason for hiding this comment

Uh oh!

clockfly Jun 2, 2016

Choose a reason for hiding this comment

Uh oh!

liancheng commented Jun 2, 2016

Uh oh!

andrewor14 commented Jun 2, 2016

Uh oh!

andrewor14 commented Jun 2, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

clockfly commented Jun 1, 2016 •

edited

Loading

clockfly Jun 2, 2016 •

edited

Loading